tests: add and follow most of the eslint rules #155
tests: add and follow most of the eslint rules #155come-maiz wants to merge 2 commits intozeppelinos:masterfrom
Conversation
| function getImplementation(string contractName) public view returns (address) { | ||
| function getImplementation(string contractName) | ||
| public view returns (address) | ||
| { |
There was a problem hiding this comment.
I don't understand why many lines are being fixed this way when we are working with a max-length of 120 Let's use 120
| "quotes": ["error", "double"], | ||
| "no-empty-blocks": "off", | ||
| "indentation": ["error", 2], | ||
| "max-len": ["error", 79], |
| "no-debugger": 0, | ||
| "no-undef": 2, | ||
| "object-curly-spacing": [2, "always"], | ||
| "max-len": [2, 120, 2], |
There was a problem hiding this comment.
is this rule shadowing the one defined in .soliumrc.json?
There was a problem hiding this comment.
sorry, this is JS... I'd fix .soliumrc.json to 120
| "no-constant": ["error"], | ||
| "security/enforce-explicit-visibility": ["error"], | ||
| "security/no-block-members": ["warning"], | ||
| "security/no-inline-assembly": ["warning"], |
There was a problem hiding this comment.
thinking about our Proxy contract, should we be warned since we are using inline-assembly to delegate calls?
There was a problem hiding this comment.
Yes, in this repo it's less useful. I like it because it forces us to have a good reason to use assembly, and we can use it as a reminder to put this good reason in a comment on the code.
I see this as a: WARNING: black magic ahead. Use with caution.
I would leave it because it's just three or four blocks of code, and hopefully it will not grow much, but no strong opinion here.
|
@facuspagnuolo I have some reasons for 80 chars here: Following the process we defied on #static, we should start using the current rules and in case of disagreement, discuss on the code-style repo. If we agree on 120, later I'll come back and adjust the code. Thanks for reviewing! |
|
Waiting for the new approach we discussed offline to tackle linter rules |
2 participants
Requires #154
Brought to you with 🤓 by the #static task force.
Remember that if you have $opinions, you should report an issue on https://github.com/ZeppelinSolutions/code-style where we will discuss about it and then update the rules accordingly, if required.